Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a protocol test for endpoints that contain path prefixes #845

Merged
merged 1 commit into from
Jun 28, 2021

Conversation

adamthom-amzn
Copy link
Contributor

Description of changes:
Sometimes, a service will have a path prefix that is not modeled. This adds a
test that ensures REST-JSON clients will properly take that path into account.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -86,5 +86,8 @@ service RestJson {
// @endpoint and @hostLabel trait tests
EndpointOperation,
EndpointWithHostLabelOperation,

// custom endpoints with paths
PathOperation,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PathOperation reads like an operation with path parameters. Maybe EndpointWithPathOperation? Or if that implies use of @endpoint trait like the couple of tests above, maybe HostWithPathOperation?

Custom endpoints supplied by users can have paths""",
protocol: restJson1,
method: "GET",
uri: "/custom/PathOperation",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uri: "/custom/PathOperation",
uri: "/custom/HostWithPathOperation",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that was a bad miss.

- The host / endpoint provided to the client, not including the path or
scheme (for example, "example.com").
- The host / endpoint provided to the client (for example, "example.com").
This will generally not include a path unless it is a non-modeled prefix,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you flip this sentence around and say when it would a path, an example of what it looks like, and how it works with paths in operations? It's important to state that here since we aren't using the same URI resolution logic defined in RFC 3986.

For example:

host MAY contain a path to indicate a base path from which each operation in the service is appended to. For example, given a host of example.com/foo/bar and an operation path of /MyOperation, the resolved host of the operation is example.com and the resolved path is /foo/bar/MyOperation.

@adamthom-amzn adamthom-amzn force-pushed the main branch 2 times, most recently from 5993114 to fdc7ad0 Compare June 28, 2021 18:41
Sometimes, a service will have a path prefix that is not modeled. This adds a
test that ensures REST-JSON clients will properly take that path into account.
@adamthom-amzn adamthom-amzn merged commit 04c26d6 into smithy-lang:main Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants